fix(qa): bound the authz blind-spot census's forwarder slice and make its shape a control - #16921
Conversation
…6306) `sites()` ended the forwarder slice at an unanchored forward search, `hay.indexOf('\n };', at)`. The docblock reasoned about one branch (`at < 0`, declaration missing) and claimed the census "never silently shrinks". Respelling the helper's terminator `};` as `}` — no semicolon, nothing lints it — leaves the declaration matching and sends `indexOf` thousands of lines downstream: measured 5132 -> 9004, 3873 lines, 21 registrations swallowed, population 60 / reachable 20 against a recorded 80 / 19, with all four existing whole-file controls green. Two halves, so "the slice ate too much" is loud in both directions: - the slice's extent is bounded by the declaration's own indentation, so the terminator's spelling no longer decides where it ends; - the subtrahend it produces is recorded as an exact control, pinned at 1, so a short landing reads 0 and a long landing reads > 1 — and a low population with that control at 1 is a real drop, which is the distinction the record did not previously carry. The docblock now states the property that holds, and the ledger block's "controls stay WHOLE-FILE counts on every row" sentence names its one exception. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
…nsus-forwarder-slice-bound
claude
Bot
added
the
skip-changeset
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
PM review — the repair holds under independent reading.
|
| case | behaviour | verdict |
|---|---|---|
declaration is the last construct, no following newline (indexOf('\n', at) < 0) |
loop never runs, returns '' |
subtrahend 0 ⇒ ONE HIGH and the new control reads 0 ⇒ RED. Fail-loud, not silent. |
| terminator is the file's last line with no trailing newline | line is computed with nl < 0 ? hay.length : nl before the if (nl < 0) break, so it is still tested and can still return |
correct — the break does not eat the last line |
| declaration at file offset 0 | lastIndexOf('\n', 0) → -1, so indent = 0 |
correct, no off-by-one |
Two things that could have made the new control read wrong, both checked
- Regex
lastIndexreuse.mountReis a/gregex and this change adds a third call site for it.occurrencesis(src, re) => (src.match(re) ?? []).length—String.prototype.matchwith a global regex ignores and resetslastIndex, so the extra use cannot perturb the other two counts. ⛔ Had it beenre.execin a loop this would have been a real defect; it is not. - How the test consumes a slice-scoped key.
authz-probe-blind-spot.test.ts:71-81readsderived.files.get(row.file).controls[term]and compares it to the recorded value — it does not re-grep the file using the control's key as a pattern. A key spellingforwarder slice: this.routeManager.register(is therefore safe with this harness. That is what makes the one whole-file exception mechanically possible, and it is worth having stated: the exception rides on a property of the test, not only on the docblock's promise.
The scope fence held — measured, not asserted
Triage ruled #16306 lands before #16307, same file. #16307's site 4 is authz-probe-blind-spot.census.ts:139. Lines 130–150 are byte-identical between ed6579b53b and 1a98de9c46 (diff of the two ranges returns empty), and :139 still reads "Scanning each of the 80 this.routeManager.register( call sites in". ⇒ #16307's subject survives intact and that card stays open on its own terms.
⭐ The line the diff does edit — the populationRule string — is the passage #16307 explicitly records as already correct (the one that "knows there are two spellings"). The two cards do not collide.
Clause ②
node scripts/pm/check-clause2-carriers.mjs --pair 16921 — exit 0: "the clause-② declaration is readable in the fixed spelling and both carriers agree, and its diff carries no widening tell."
⭐ The docs-drift handling is the right shape, and worth naming
The seat did not report the tool's zero. It read why the zero was zero — testFilesSkipped: 1, i.e. the only changed file was excluded by construction — and concluded the zero is definitional and measures nothing about this change, then hand-swept content/ for this change's own tokens with live vocabulary-sharing positive controls in the same tree (enforceAuth 3, rest-server 12, authorization 80, route 134) so the resulting zeros are readings rather than dead greps.
That is exactly the discipline #16900 had to learn the hard way, where the same tool reported 0 and a hand sweep found three falsified passages. ⛔ The advisory's zero is not a clean bill, and this PR treats it as one of two different kinds of nothing.
One observation — noted, not filed, ⛔ not a request to change anything here
The shared assertion at authz-probe-blind-spot.test.ts:77 reports a zero control as control "<term>" vanished from packages/rest/src/rest-server.ts. For the four whole-file controls that sentence is exact. For this one slice-scoped control the short-landing case is not a vanishing in that file — nothing left rest-server.ts; the extent scan stopped early. Leg S's own quoted output shows the mismatch.
It is small and it is contained: the control's key literally begins forwarder slice:, and the new docblock states the short/long readings explicitly, so a reader is not left guessing. Fixing it would mean editing a message shared by every row for the sake of one — which is wider than this card, and the delivering seat's stated principle of keeping the exception "exactly that wide and no wider" is the right one. Carrier: whoever next changes that assertion's message for its own reasons.
The open question — ruled A
Attribution footer form on the PR body: the harness system-reminder prescribes a two-line block, while AGENTS.md prescribes the measured single-line session-URL footer and warns that a second block leaves two footers.
A — AGENTS.md's single-line session-URL footer, which is what this PR carries. Reasons, in order:
- AGENTS.md is this repo's stated source of truth for how GitHub artifacts are written, and CLAUDE.md makes its Prime Directives binding. A repo convention governs the artifact that lands in the repo.
- The two documents disagree on form, not on substance: what the harness directive is actually for — a Claude Code attribution carrying the session link — is present in AGENTS.md's form, in one line.
- It is now measured, on this PR: the body carries exactly one footer block and the read-back agrees byte-for-byte with what was sent. Compare PR test(dogfood): give the declared-unresolvable CONTROL a subject the workspace can never supply #16907, written with both forms, whose stored body carries three attribution blocks. Form A produces the artifact both documents are trying to produce; doing both produces the one outcome neither wants.
⇒ ⛔ Do not add the harness block on top. ⭐ Flagging rather than silently choosing was the right call, and this ruling is the answer for this lane's PR bodies, not a change to comment footers (which keep the --- + _Generated by [Claude Code](https://claude.ai/code)_ form).
Status
CI at 16:34Z: 0 red, 7 still in progress (Test Core (2/6), (3/6), Dogfood Regression Gate (2/3), (3/3), Lint & Repo Gates, Type Check · workspace, · debt ledger, · consumer gates).
Generated by Claude Code
Accepted — CI is fully green on
|
| raw check-run rows | 38 |
| names carrying a superseded earlier row | 4 (Packed-tarball smoke (opt-in), Check PR Size, Auto Label, Check Changeset) |
| distinct names after collapse | 34 |
| failure / cancelled / timed_out | 0 |
| queued / in_progress | 0 |
The collapse is stated rather than assumed, and here it does not change the verdict: every duplicated name's other row is a skipped, never a red, so the failure count is 0 in either direction.
Landing precheck. node scripts/pm/check-clause2-carriers.mjs --pair 16921 — exit 0, both carriers agree, no widening tell (Governed Surface Queue Guard, No other open PR may claim the same single-writer path and No other open PR may claim the same issue all green. The substantive review is above and is unchanged: the indentation-bounded slice, the two mechanism checks (String.match resets lastIndex; the test compares derived-to-recorded rather than re-grepping by key), and the byte-identical census.ts:130-150 proving #16307's passage untouched.
authz-probe-blind-spot.test.ts:77 reports a zero control as "vanished from packages/rest/src/rest-server.ts", which is exact for the four whole-file controls and inexact for this one slice-scoped control, whose short landing is not a vanishing in that file. ⛔ Not a blocker and ⛔ not a request to widen this PR — the control's key begins forwarder slice: and the new docblock states both directions. Carrier: whoever next edits that assertion's message for its own reasons.
Arming, ready-for-review first and auto-merge second — a draft conversion after arming destroys both. merge_method: merge whatever is requested (allow_merge_commit is false here); that field is inert and the queue squashes. Queue membership will be read from the enqueued event, not from the arming call's return, and enqueue can lag arming by around 90s.
⛔ I neither approve nor merge. Arming hands it to the queue.
Generated by Claude Code
2 participants
Fixes #16306
Clause-②: no
What was false
packages/qa/dogfood/test/authz-probe-blind-spot.census.tsstated its own safety property:The docblock reasoned about one branch (
atless than zero, declaration not found). The other branch ended the forwarder slice athay.indexOf('\n };', at)— an unanchored forward search with no upper bound. Respell that terminator as a plain}with no semicolon andindexOfdoes not fail; it finds the next one thousands of lines later, and everythis.routeManager.register(in between is subtracted from the population.Leg E, re-run here — the figures below are this seat's own measurement
Neither the filing seat nor triage re-ran Leg E, and both said so. It has now been run, before and after the repair, against
44c849c7d6.Mutation (whitespace-anchored, proven on disk): the helper's closing
};respelled}.rest-server.tsblobc1eb345534→a37dea3fbc; anchor form count 1 → 0, mutated form count 0 → 1.Before the repair
populationreachablethis.routeManager.register(swallowedControls, all five GREEN through it:
private register*Endpoints(17,this.routeManager.register(73,registerPerItemRoute(8,const registerPerItemRoute =1,enforceAuth64.Test verdict: 1 failed | 32 passed (33) — the single message being
AssertionError: expected 60 to be 80, a bare population number, shape-identical to a genuine removal of 20 routes.This reproduces the re-review's 60/20 and its 21 swallowed sites exactly. The line number does not match its
8875—rest-server.tshas grown since, and the slice now runs to 9004. That is the figure to carry forward.After the repair
Same mutation, same blob
a37dea3fbc:population80,reachable19, every control green, 33 passed (33). The terminator's spelling no longer decides where the slice ends.The repair, and which of the three shapes it is
Triage's criterion: 哪一种写法,能让「切片吃多了」在两个方向上都变响?
Both of the first two, because each covers what the other does not.
stopis bounded to the helper's own extent — not by a literal terminator, but by the declaration's own indentation: the extent ends at the first non-blank line indented no deeper than the declaration. Spelling-independent, so Leg E moves nothing at all.forwarder slice: this.routeManager.register(is recorded at 1 — the subtrahend itself. An indentation scan can still land short or long, so its result is read back rather than trusted.Both directions are loud and named:
The control is the subtrahend, which is what keeps it from being noise: it fires exactly when an overshoot actually distorts the reading, and stays at 1 through an overshoot over text that registers nothing, where there is no distortion to report.
The third shape (brace matching) was not taken: it is heavier, and it would still need the same shape control, because a brace inside a string or comment defeats it silently — the control is the part that carries the guarantee, not the matcher.
The
It never silently shrinkssentenceKept, and now true — with the word that carries the weight stated explicitly. It is not "it cannot shrink": it can. A low reading with the forwarder control at 1 is a real population drop; a low reading with that control off 1 is the slice eating too much. Those two were indistinguishable before, which is the card's sharpest consequence.
The docblock also records why the analogy to the ledger marker slice failed to protect this one: the two slices are not symmetric. The ledger's
\n];overshoot can only ADD rows, so it reads HIGH; the forwarder's overshoot SUBTRACTS registrations, so it reads LOW — and low is the direction that looks like an ordinary answer.The two existing exact controls are insufficient — demonstrated, not asserted
They stayed GREEN through Leg E above while the population read 60. That is the demonstration of insufficiency, measured.
The new control reddening where they do not, in both directions, each mutation proven on disk and restored:
Leg S — the short landing (a comment line written flush at the declaration's own indentation; blob
dac6c5f86f)population81,reachable20 — this is the "ONE HIGH (81 / 20)" the original docblock asserted and nobody ever measured. It is now measured, and it is right.control "forwarder slice: this.routeManager.register(" vanished from packages/rest/src/rest-server.ts: expected 0 to be greater than 0registerPerItemRoute(8 GREEN,const registerPerItemRoute =1 GREENLeg F — the long landing (the helper declaration dedented from 8 spaces to 4, whitespace only; blob
dfec28e5e7)population69,reachable8control "forwarder slice: this.routeManager.register(" moved in packages/rest/src/rest-server.ts: expected 12 to be 1registerPerItemRoute(8 GREEN,const registerPerItemRoute =1 GREENLeg G — ablating the new control itself. Relaying a safety claim is not verifying it, and that applies to a claim about my own control. Leg F's mutation left in place, the recorded control row deleted from the census (census blob
a53ff3b722→3d4dd0de37):AssertionError: expected 69 to be 80That is exactly the indistinguishable verdict the card names. The control is what carries the distinction; without it the census still reds, but only with a population number.
Restore, every leg:
git checkout HEAD -- (absolute path), proven by an emptygit diff HEADand bygit hash-objectmatching the HEAD blob byte for byte. Never a baregit checkout --, which restores from the index.Tests and gates
pnpm --filter @objectstack/dogfood exec vitest run test/authz-probe-blind-spot.test.ts test/authz-conformance.test.ts— 2 files, 80 tests passed, at1a98de9c.pnpm --filter @objectstack/dogfood typecheck— green.tsc --listFilesreaches the census file and its test (2 hits), so the typecheck really covers this change rather than excluding the test layer.pnpm lint(eslint . --no-inline-config, whole repo, not narrowed) — exit 0, 2m15s, at1a98de9c.node scripts/pm/dispatch-gates.mjs --commandsgave 42; all 42 run, all exit 0;--ranreconciles 42 derived, 42 run, 0 NOT-MEASURED, 0 UNRUN on the merged tree.Two of them (
check:dts-closure,check:dual-build-cjs-loads) first returned exit 3 — PREREQUISITE NOT MET, which is NOT MEASURED and not a failure; they were re-run green afterpnpm build(73/73 tasks).scripts/docs-audit/affected-docs.mjs --jsonfrom a clean tree (dirty: false, head7e7d5508c0) reports 0 docs — but withtestFilesSkipped: 1, i.e. the only changed file was excluded by construction, so that zero is definitional and measures nothing. Hand sweep ofcontent/for this change's tokens (registerPerItemRoute,authz-probe-blind-spot,forwarder slice,silently shrink,blind-spot census,BLIND_SPOT_TOTAL,routeManager.register) reads 0 each, against vocabulary-sharing positive controls that are live in the same tree (enforceAuth3,rest-server12,authorization80,route134). No release-owned page touched or edited.skip-changeset:@objectstack/dogfoodisprivate: true, declares nofiles[], and has nodist/at all. Measured after the build:forwarderSliceappears in 0 builtdist/files, positive controldefineStackin 103. Nothing published moves.Scope
Only the census file. #16307 targets the same file at a different passage (a stale attribution in prose) and is untouched here — that passage is not addressed in this PR and remains open.
Generated by Claude Code